-
Notifications
You must be signed in to change notification settings - Fork 8
Add new Jip, separate Tom and Floor devices #842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughStandardizes device model strings from "Tom/Floor" to "Tom" across fixtures and tests, updates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Solution for home-assistant/core#159741 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #842 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 3435 3435
=========================================
Hits 3435 3435 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
CHANGELOG.mdfixtures/adam_jip/data.jsonfixtures/adam_multiple_devices_per_zone/data.jsonfixtures/adam_plus_anna_new/data.jsonfixtures/adam_plus_anna_new_regulation_off/data.jsonfixtures/adam_zone_per_device/data.jsonfixtures/m_adam_cooling/data.jsonfixtures/m_adam_heating/data.jsonfixtures/m_adam_jip/data.jsonfixtures/m_adam_multiple_devices_per_zone/data.jsonplugwise/constants.pytests/data/adam/adam_jip.jsontests/data/adam/adam_multiple_devices_per_zone.jsontests/data/adam/adam_plus_anna_new.jsontests/data/adam/adam_plus_anna_new_regulation_off.jsontests/data/adam/adam_zone_per_device.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-11T14:05:29.022Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 797
File: fixtures/adam_anna_new_2/data.json:1-304
Timestamp: 2025-10-11T14:05:29.022Z
Learning: In Plugwise fixture data (data.json files), location IDs referenced in device entries don't always need corresponding top-level location objects. Only climate zones (ThermoZone entries with dev_class "climate") require full top-level definitions with thermostats, schedules, and presets. Simple organizational locations that just group devices (like single plugs) can be referenced without top-level entries.
Applied to files:
fixtures/m_adam_jip/data.jsonfixtures/m_adam_heating/data.jsontests/data/adam/adam_multiple_devices_per_zone.jsontests/data/adam/adam_zone_per_device.jsonfixtures/m_adam_cooling/data.jsonfixtures/adam_multiple_devices_per_zone/data.jsonfixtures/m_adam_multiple_devices_per_zone/data.jsonfixtures/adam_zone_per_device/data.json
📚 Learning: 2024-12-22T09:37:24.648Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 678
File: plugwise/legacy/helper.py:69-69
Timestamp: 2024-12-22T09:37:24.648Z
Learning: The code in plugwise/legacy/helper.py is fully separated from plugwise/helper.py. The user indicates that the problem signaled about the mismatch of the variable name (_home_location vs _home_loc_id) does not actually apply.
Applied to files:
plugwise/constants.py
📚 Learning: 2025-12-25T12:02:58.781Z
Learnt from: bouwew
Repo: plugwise/python-plugwise PR: 835
File: plugwise/helper.py:837-841
Timestamp: 2025-12-25T12:02:58.781Z
Learning: In the Python Plugwise library, zones are not separate appliances in gw_entities. When a zone is added to self.gw_entities, the location_id is used as the entity_id. In code reviews, validate that zone checks rely on location_id being present in _existing_zones and that lookups use self.gw_entities[location_id]. Do not treat zones as independent appliances.
Applied to files:
plugwise/constants.py
🔇 Additional comments (9)
fixtures/adam_plus_anna_new/data.json (1)
74-74: LGTM! Model name standardization looks correct.The model field update from "Tom/Floor" to "Tom" for model_id "106-03" aligns with the HW_MODELS mapping change in constants.py. This is part of the systematic refactoring to separate combined device models into distinct types.
fixtures/adam_plus_anna_new_regulation_off/data.json (1)
48-48: LGTM! Consistent model name update.The model field correctly updated to "Tom" for this thermostatic radiator valve, matching the HW_MODELS mapping change.
fixtures/m_adam_heating/data.json (1)
68-68: LGTM! Model standardization applied correctly.Consistent update matching the HW_MODELS change for model_id "106-03".
fixtures/m_adam_cooling/data.json (1)
63-63: LGTM! Model field updated correctly.Consistent with the model naming standardization across all fixtures.
fixtures/m_adam_jip/data.json (1)
87-87: LGTM! All Tom device models updated consistently.All four thermostatic radiator valves with model_id "106-03" correctly updated from "Tom/Floor" to "Tom", maintaining consistency with the HW_MODELS mapping change.
Also applies to: 111-111, 178-178, 278-278
tests/data/adam/adam_plus_anna_new_regulation_off.json (1)
48-48: LGTM! Test data updated to match fixtures.The model field correctly updated in test data to align with the fixture changes and constants mapping.
tests/data/adam/adam_plus_anna_new.json (1)
74-74: LGTM! Test data synchronized with fixture updates.The model field correctly updated to maintain consistency between test data and fixture files.
plugwise/constants.py (1)
53-58: New model IDs "165-01" and "168-02" not found in test fixtures.The model mapping updates look good. The "106-03" → "Tom" change correctly aligns with the fixture data changes throughout the PR. However, neither "165-01" (Floor) nor "168-02" (Jip variant) appear anywhere in the existing test fixtures. The variant "168-01" for Jip already has test coverage in multiple fixtures, making these new additions represent untested hardware variants. Confirm these model IDs correspond to actual Plugwise device versions before merging.
⛔ Skipped due to learnings
Learnt from: CoMPaTech Repo: plugwise/python-plugwise PR: 698 File: fixtures/m_adam_multiple_devices_per_zone/data.json:21-21 Timestamp: 2025-01-29T19:14:31.257Z Learning: MAC addresses in test fixtures of the python-plugwise repository are mock addresses and do not represent real device information.CHANGELOG.md (1)
3-6: LGTM! Clear documentation of ongoing work.The addition of the "Ongoing" section appropriately documents the model-data standardization effort for Jip, Tom, and Floor devices.
|



Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.